Skip to content

Add diagnosis APIs to MediaWiki benchmark#552

Open
marziehlenjaniMeta wants to merge 1 commit intofacebookresearch:v2-betafrom
marziehlenjaniMeta:export-D98805269-to-v2-beta
Open

Add diagnosis APIs to MediaWiki benchmark#552
marziehlenjaniMeta wants to merge 1 commit intofacebookresearch:v2-betafrom
marziehlenjaniMeta:export-D98805269-to-v2-beta

Conversation

@marziehlenjaniMeta
Copy link
Copy Markdown

Summary:
This diff adds reusable pre-flight diagnostic checks and centralizes diagnosis
merging at the framework level, so all benchmarks benefit automatically.

1. Shared check functions (packages/common/diagnosis_utils.py)
Added two reusable check functions to the shared diagnosis API:

  • check_file_descriptor_limit(): verifies ulimit is sufficient, with optional
    auto-fix that records the change under notable_auto_fixes
  • check_selinux(): detects SELinux Enforcing mode (causes HHVM segfaults)

These join existing check_ipv6_hostname() and check_port_available().

2. Generic pre-flight CLI (packages/common/preflight_checks.py)
A reusable script callable from any benchmark's bash runner:

  • Accepts --benchmark <name> and --min-fds <N> for per-benchmark config
  • Runs SELinux, IPv6 hostname, and file descriptor checks
  • Fixes inverted check_ipv6_hostname usage (returns True when IPv6 is
    detected, not necessarily broken — should not gate all_ok)

3. Framework-level diagnosis merge (benchpress/cli/commands/run.py)
After job.run(), run.py now automatically merges DiagnosisRecorder records
(failures + auto-fixes) into metrics via DIAGNOSIS_FILE_PATH. Every benchmark
gets diagnosis merging for free — no per-benchmark merge code needed. Removed
the now-redundant merge_failure_to_results() call from tao_bench's
run_autoscale.py.

4. MediaWiki integration

  • run.sh: Calls common preflight_checks.py before benchmark; raises
    ulimit -n in bash so child processes (HHVM, nginx, wrk) inherit the limit
  • jobs.yml: Added -U {auto_fix_ulimit} arg with default auto_fix_ulimit=1
    to all 5 mediawiki job entries

Differential Revision: D98805269

Summary:
This diff adds reusable pre-flight diagnostic checks and centralizes diagnosis
  merging at the framework level, so all benchmarks benefit automatically.

  **1. Shared check functions (`packages/common/diagnosis_utils.py`)**
  Added two reusable check functions to the shared diagnosis API:
  - `check_file_descriptor_limit()`: verifies ulimit is sufficient, with optional
    auto-fix that records the change under `notable_auto_fixes`
  - `check_selinux()`: detects SELinux Enforcing mode (causes HHVM segfaults)

  These join existing `check_ipv6_hostname()` and `check_port_available()`.

  **2. Generic pre-flight CLI (`packages/common/preflight_checks.py`)**
  A reusable script callable from any benchmark's bash runner:
  - Accepts `--benchmark <name>` and `--min-fds <N>` for per-benchmark config
  - Runs SELinux, IPv6 hostname, and file descriptor checks
  - Fixes inverted `check_ipv6_hostname` usage (returns True when IPv6 is
    detected, not necessarily broken — should not gate `all_ok`)

  **3. Framework-level diagnosis merge (`benchpress/cli/commands/run.py`)**
  After `job.run()`, `run.py` now automatically merges DiagnosisRecorder records
  (failures + auto-fixes) into metrics via `DIAGNOSIS_FILE_PATH`. Every benchmark
  gets diagnosis merging for free — no per-benchmark merge code needed. Removed
  the now-redundant `merge_failure_to_results()` call from tao_bench's
  `run_autoscale.py`.

  **4. MediaWiki integration**
  - `run.sh`: Calls common `preflight_checks.py` before benchmark; raises
    `ulimit -n` in bash so child processes (HHVM, nginx, wrk) inherit the limit
  - `jobs.yml`: Added `-U {auto_fix_ulimit}` arg with default `auto_fix_ulimit=1`
    to all 5 mediawiki job entries

Differential Revision: D98805269
@meta-cla meta-cla bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Apr 1, 2026
@meta-codesync
Copy link
Copy Markdown

meta-codesync bot commented Apr 1, 2026

@marziehlenjaniMeta has exported this pull request. If you are a Meta employee, you can view the originating Diff in D98805269.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported meta-exported

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant